Greptile Summary

This PR adds special-case dependency-string generation for Argo Workflows DAG tasks where a static-split join step is nested inside a conditional (split-switch) branch. Without the fix the generated depends expression could be incorrect, potentially allowing the join step to execute before all of its predecessor steps have completed.

Key changes and observations:

• A new block is inserted after the existing _is_conditional_skip_node / _many_in_funcs_all_conditional overrides. It activates when node.type == "join" and at least one in_func is a conditional node.
• Two inner helpers are introduced: _split_switch_ancestors (recursive backwards traversal to collect split-switch ancestors up to the enclosing static-split boundary) and build_ancestor_tree (groups in-funcs by their frozenset of conditional ancestors, then chains subsets greedily to produce a nested dependency expression).
• The isinstance(g, list) guard in build_ancestor_tree is dead code — node_groups values are always flat lists of step-name strings, so the branch that handles a nested list can never be reached.
• No unit tests are added for the new algorithm. The only existing Argo unit test file (test/unit/test_argo_workflows_cli.py) covers only sanitize_for_argo, leaving the new grouping/chaining logic untested at the unit level.
• Several correctness edge-cases (empty split_parents, missing visited set in recursion, operator choices for the emitted expression) were discussed extensively in prior review rounds.

Confidence Score: 2/5

• Not safe to merge — the new logic contains dead code, has no unit-test coverage, and several correctness concerns raised in prior rounds remain unresolved.
• The algorithm is non-trivial and operates on a critical path (Argo depends expressions directly control task scheduling). The dead isinstance(g, list) branch is evidence of unvetted code. The recursive _split_switch_ancestors helper has no visited set (flagged previously) and both inner helpers are entirely untested. Multiple prior review comments about operator correctness and edge-case handling were acknowledged but not fully resolved in the diff.
• metaflow/plugins/argo/argo_workflows.py — specifically the new block from line 1415 to 1497, _split_switch_ancestors, and build_ancestor_tree.

Important Files Changed

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    start --> switch_step
    switch_step -->|route == a| branch_a
    switch_step -->|route == b| branch_b
    branch_a -->|static split| sub_a
    branch_a -->|static split| sub_b
    sub_a --> sub_join["sub_join (join)"]
    sub_b --> sub_join
    sub_join --> shared["shared (conditional join)"]
    branch_b --> shared
    shared --> end_step[end]

    subgraph "New depends generation for sub_join"
        NB1["1. Detect: node.type==join AND any in_func is conditional"]
        NB2["2. Build node_groups by split_branches[-1]\n{branch_a: [sub_a, sub_b]}"]
        NB3["3. _split_switch_ancestors(fn, split_parents[-1])\ncollects any split-switch nodes between fn and the static split"]
        NB4["4. build_ancestor_tree groups by frozenset of switch ancestors\nand chains subsets together"]
        NB5["5. Emit depends string from required_deps\ne.g. ((sub-a.Succeeded || sub-b.Succeeded))"]
        NB1 --> NB2 --> NB3 --> NB4 --> NB5
    end

_{Last reviewed commit: 8939b41}